Skip to content

Retry credential fill after transient Git config access failures - #2150

Merged
Tyrie Vella (tyrielv) merged 4 commits into
microsoft:masterfrom
tyrielv:tyrielv/first-logon-mount-recovery
Nov 3, 2026
Merged

Tyrie Vella (tyrielv) merged 4 commits into
microsoft:masterfrom
tyrielv:tyrielv/first-logon-mount-recovery

Conversation

@tyrielv

@tyrielv Tyrie Vella (tyrielv) commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem and Context

Mount runs credential retrieval and local Git config writes in parallel. On Windows, a config replacement can make a concurrent reader report Permission denied.

git credential fill reads Git config before it returns credentials. VFS for Git treats a transient config-read failure as a credential failure. A mount with a cache server can then continue without the server config.

A timed process wait can also return before redirected output callbacks finish. The retry classifier can then miss the config error, or a successful fill can lose its credential output.

PR #2130 retries explicit Git config reads. This change covers credential fill, which also reads config. It does not depend on #2130.

Changes

  • Retry credential fill only for a failed, untruncated result with empty stdout. Stderr must contain only the supported English Git config-read diagnostics. The access warning must report Permission denied or Device or resource busy.
  • Share the fill retry path between repository credentials and certificate passwords. Make at most five attempts, with delays of 20, 40, 80, and 160 milliseconds.
  • Deduct elapsed time from the process timeout before each retry. Stop retries when the remaining budget cannot cover the delay.
  • Preserve successful fills, missing-password results, other credential failures, and the final error message. Do not retry credential approval or rejection.
  • Drain redirected stdout and stderr after Git exits, before the shared process runner returns the result. Leave the timeout-failure path unchanged.
  • Emit CredentialConfigReadFailure for each matching failed attempt, including exhaustion. WillRetry distinguishes a retry from the final failure. Use RetryDelayMilliseconds for the delay.
  • Keep config paths, URLs, credential values, and stderr out of retry metadata. Keep stderr in the local log.
  • Add unit tests for recovery, exhaustion, non-retryable failures, truncation, timeout budgets, and both fill callers.
  • Add unit tests with real Git and an exclusive config-file lock, for both finite and infinite timeouts. The tests release the lock after the first failure or retain it through exhaustion. They use a local helper with test credentials.
  • Add unit tests for output completion after process exit. The tests pause a real Git output callback and verify that timed and infinite waits await its completion.

Retry only complete, empty-output failures that report both a transient
file-access warning and Git's config-read failure. Apply the same policy
to repository credentials and certificate passwords.

Bound the retry count and preserve the remaining process timeout.
Report retries and exhaustion without credential data or stderr.

Add focused unit tests and real Git config-lock regression tests.

Assisted-by: GPT-6.1 Sol
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Reject additional stderr diagnostics so a helper refusal does not match
the config-read retry policy.

Name the event for a config-read failure, including non-retryable
exhaustion. Align the delay field with the config-read retry schema.

Add assertions for infinite timeouts, busy-file errors, CRLF diagnostics,
retry metadata fields, and timeout results after a config-read failure.

Assisted-by: GPT-6.1 Sol
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Reject a transient access warning when Git also reports a non-transient config failure.

Assisted-by: GPT-6.1 Sol

Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv
Tyrie Vella (tyrielv) marked this pull request as ready for review November 2, 2026 18:56
@KeithIsSleeping

Copy link
Copy Markdown

[KeithBot]: ⚠️ 1 issue · iteration 1 (d9b1b9a)

  1. 🟡 Medium Config-read retry skipped when timed WaitForExit returns before stderr

Validation

  • ❌ PR credential fixtures (50 tests)
  • ❌ Locked-fill probe, 30 s timeout
  • ✅ Build: unit tests, natives, Mount
  • ✅ Full unit suite at source + base
  • ✅ Real-git race + loopback HTTP auth
  • ✅ Mutants and base-code red control
  • ✅ CI checks on head commit
  • ⏭️ Functional tests and installer — not run

🤖 KeithBot · claude-opus-5.5, gpt-6.1-sol, gemini-4-argon · reply on a thread to dispute

A successful timed WaitForExit does not wait for redirected stdout and
stderr callbacks. Drain those callbacks after Git exits so credential
config-read failures and successful credential output reach their callers.
Leave the timeout-failure path unchanged.

Add a real-Git regression test that pauses an output callback. The timed
case fails without the drain, while the infinite-wait case remains valid.
Exercise locked-config credential fills with finite and infinite timeouts.

Assisted-by: GPT-6.1 Sol
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@KeithIsSleeping

Copy link
Copy Markdown

[KeithBot]: ✅ Resolved · iteration 2 (baabb71)

  • Config-read retry skipped when timed WaitForExit returns before stderr: fixed
  • New changes since iteration 1: no new issues

Validation

  • ⚠️ Stderr-holding helper, base vs source — partial
  • ✅ PR fixtures and auth tests at source
  • ✅ Locked first fill, 16-way, 30 s and -1
  • ✅ Full unit suite (r1, r3 runs)
  • ✅ CI checks on head commit
  • ⏭️ Functional tests and installer — not run

Approving.

🤖 KeithBot · claude-opus-5.5, gpt-6.1-sol, gemini-4-argon · reply on a thread to dispute

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the current head: the previous concerns are addressed and current-head validation passed.

@tyrielv
Tyrie Vella (tyrielv) merged commit 433ffe3 into microsoft:master Nov 3, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants